Skip to content

Allow fetch prerelease #495

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 5 commits into from

Conversation

crossan007
Copy link

@crossan007 crossan007 commented Dec 28, 2016

closes #494

@Nyholm
Copy link
Collaborator

Nyholm commented Dec 29, 2016

Thank you for this PR. I like that you added both documentation and test.
Correct me if Im wrong but this allows the user to do:

$latestRelease = $api->latestIncludingPrereleases('username', 'repo');

Wouldn't the result be the same if you did:

$latestRelease = $api->all('username', 'repo')[0];

If so, I do not believe we need this PR. The API client should reflect the API and the API does not have a endpoint for getting the latest release including prereleases and drafts.

Other maintainers/contributors may have different opinion though?

@crossan007
Copy link
Author

Thanks for your reply - Your interpretation is correct.

I just thought it would be nice to have an aptly named function to access the requested data.

If nothing else, this PR (and issue #494) will serve as a reference for other users of this project who may want to access pre-releases within their application.

@Nyholm
Copy link
Collaborator

Nyholm commented Dec 29, 2016

What do you think about just updating the documentation and not the code? The documentation can show how you get the latest release including prereleases.

@crossan007
Copy link
Author

Sure. I can update this PR

@@ -9,6 +9,12 @@ Provides information about releases for a repository. Wraps [GitHub Releases API
$release = $client->api('repo')->releases()->latest('twbs', 'bootstrap');
```

The ```latest()``` method fetches only releases which are not marked "prerelease."
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should only use one backtick at each side of the word.
`latest()`

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"prerelease" and "draft".

@@ -9,6 +9,12 @@ Provides information about releases for a repository. Wraps [GitHub Releases API
$release = $client->api('repo')->releases()->latest('twbs', 'bootstrap');
```

The ```latest()``` method fetches only releases which are not marked "prerelease."

To obtain the latest release *including prereleases*, select the first element in the "all releases" function:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

*including* prereleases and drafts,

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

auth is required for drafts. I've added that to the doc also

The ```latest()``` method fetches only releases which are not marked "prerelease."

To obtain the latest release *including prereleases*, select the first element in the "all releases" function:
```
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

```php

Copy link
Collaborator

@Nyholm Nyholm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Excellent. Just a minor thing before I can merge

@@ -9,6 +9,15 @@ Provides information about releases for a repository. Wraps [GitHub Releases API
$release = $client->api('repo')->releases()->latest('twbs', 'bootstrap');
```

The `latest()` method fetches only releases which are not marked "prerelease" or "draft"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing period.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bump, can you fix this minor so I can merge?

@Nyholm Nyholm mentioned this pull request Feb 1, 2017
@Nyholm
Copy link
Collaborator

Nyholm commented Feb 1, 2017

Closed by #519

@Nyholm Nyholm closed this Feb 1, 2017
Nyholm added a commit that referenced this pull request Feb 1, 2017
* allow fetching prerelease data

* fix line endings

* use 0th element for test

* only update documentation

* update docs by request. add draft auth detail

* fixed typo
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Get Latest Pre-Release
3 participants